Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checklist points and card points update #79

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

onilton
Copy link

@onilton onilton commented Feb 21, 2015

No description provided.

@onilton
Copy link
Author

onilton commented Feb 21, 2015

I used @mgan59 's commit as the base to submit a new pull request that tries to accomplish all the things attempted to fullfill in this pull request #22.

It was heavily inspired by #22 but a LOT of changes were necessary to make it work
with current trunk.

But now you can also:

  • Choose estimated and consumed points in the checklist.
  • Consumed and estimated points are updated in the card title when points are given to checklist items

@SeanColombo
Copy link
Contributor

Screenshot of it in action:
http://screencast.com/t/51apuvwNO2Q

This PR also adds the consumed-points picker to the card title, which was in another PR but which hadn't been merged.

@onilton
Copy link
Author

onilton commented Feb 26, 2015

Consumed-points picker for card title is not in master?

I thought it was:
https://github.com/Q42/TrelloScrum/blob/master/trelloscrum.js#L710

Or am I missing something? :)

@SeanColombo
Copy link
Contributor

Awesome, yeah it is in master. I guess it hadn't been pushed until today? I didn't have it on this computer yesterday, unless I'm mistaken. Cool beans :)

@onilton
Copy link
Author

onilton commented Mar 4, 2015

Any idea on how this pull request could be improved?

mgan59 and others added 9 commits March 5, 2015 00:33
Heavily inspired by the pull request Q42#22:
Q42#22
But a LOT of changes were necessary to make it work
with current trunk.
This was a mistake of the original code from pull request Q42#22.
We were iterating on pointSeq (Fibonacci numbers). Actually we were
iterating on the indices of this array, so we were stopping at the 9th
item of the list always.
Now we are iterating in all checklists items.
@onilton onilton force-pushed the checklist-points branch from 302417b to b5210cb Compare March 5, 2015 03:33
@SeanColombo
Copy link
Contributor

@oniltonmaciel it works really well. I think the only question is conceptual (whether it decreases the simplicity)

@jkaizer: what do you think? If it's potentially too much, maybe we could make it togglable behind a Setting?

@jkaizer
Copy link
Member

jkaizer commented Mar 6, 2015

Awesome work!

I am thinking this should be behind a setting where the default setting disables this functionality.
Is that easy to do?

@mgan59
Copy link
Contributor

mgan59 commented Mar 8, 2015

@oniltonmaciel Been on vacation and traveling, this looks cool. Will pull it down and give it a try. Cheers!

@Kudo
Copy link

Kudo commented Jun 23, 2015

+1 and look forward to seeing this to be published

@SeanColombo
Copy link
Contributor

This still had a few edge-cases if I remember correctly.

Perhaps we should QA & Polish it a bit, then hide it behind a setting?

I just noticed that one of the github Issues for our project, is actually a bug in this PR rather than in the released code: #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants